Skip to content

Chat 20260514 130223#306

Merged
arul28 merged 2 commits into
mainfrom
ade/chat-20260514-130223-bc1f1152
May 14, 2026
Merged

Chat 20260514 130223#306
arul28 merged 2 commits into
mainfrom
ade/chat-20260514-130223-bc1f1152

Conversation

@arul28

@arul28 arul28 commented May 14, 2026

Copy link
Copy Markdown
Owner

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

Greptile Summary

This PR introduces a pending-image-attachment UX: while clipboard or file images are being saved to disk, the attachment tray shows a cancellable thumbnail so the send button is disabled until the save completes. A new appSaveClipboardImageAttachment IPC handler reads the Electron clipboard, writes the PNG to .ade/attachments/, and returns a downsized 96×96 preview, avoiding the previous pattern of routing the full base64 payload through the renderer.

  • New IPC handler (appSaveClipboardImageAttachment): reads clipboard PNG in the main process, saves it with the shared saveAgentChatTempAttachmentBuffer helper, and returns a compact preview data URL instead of the full image bytes.
  • Pending-attachment state (pendingImageAttachments, imagePreviewUrls): composer tracks in-flight saves; ChatAttachmentTray renders a PendingImageAttachmentPreview spinner with a blob-URL thumbnail while the temp file write is in progress.
  • Preview seeding: once a save completes, the resulting ImageAttachmentPreview receives the already-computed URL as initialPreviewUrl, skipping the async getImageDataUrl IPC round-trip entirely.

Confidence Score: 5/5

Safe to merge — no functional regressions identified; the pending-attachment flow is guarded correctly at every async boundary.

The new IPC handler and React state machinery are each internally consistent. The cancellation ref is checked after every await, the object URL lifecycle is managed through effects with correct prev/current diffing, and the composerInputLocked effect's early-return guard prevents the pendingImageAttachments dep from causing spurious cancellations. The one suggestion is a dep-array tidyup that doesn't change runtime behavior.

No files require special attention; the most complex logic is in AgentChatComposer.tsx but its pending-attachment state machine is well-tested by the new test cases.

Important Files Changed

Filename Overview
apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx Introduces pending image attachment state, preview URL lifecycle management, and the saveClipboardImageAttachment native path; composerInputLocked effect adds pendingImageAttachments to deps unnecessarily (guard makes it harmless but adds noise)
apps/desktop/src/main/services/ipc/registerIpc.ts Adds saveAgentChatTempAttachmentBuffer helper and appSaveClipboardImageAttachment IPC handler; refactors agentChatSaveTempAttachment to share the helper; all paths properly async and size-checked
apps/desktop/src/renderer/components/chat/ChatAttachmentTray.tsx Adds ChatAttachmentPendingImage type and PendingImageAttachmentPreview component; ImageAttachmentPreview accepts an initialPreviewUrl to skip the async getImageDataUrl call when a seeded blob URL is available
apps/desktop/src/main/services/adeActions/registry.ts Converted saveAgentChatTempAttachment from sync to async (fs.promises.*) — straightforward correctness improvement
apps/desktop/src/preload/preload.ts Exposes saveClipboardImageAttachment via contextBridge, matching the new IPC handler signature exactly
apps/desktop/src/renderer/browserMock.ts Adds saveClipboardImageAttachment stub (resolved null) alongside existing clipboard mocks — complete
apps/desktop/src/shared/ipc.ts Adds appSaveClipboardImageAttachment IPC channel constant — minimal, correct

Sequence Diagram

sequenceDiagram
    participant User
    participant Renderer as AgentChatComposer
    participant Tray as ChatAttachmentTray
    participant Main as Main Process (IPC)
    participant FS as Filesystem

    User->>Renderer: Cmd+V / Paste / Drop image
    Renderer->>Renderer: addPendingImageAttachment()
    Renderer->>Tray: render PendingImageAttachmentPreview

    alt Native clipboard path
        Renderer->>Main: IPC: appSaveClipboardImageAttachment
        Main->>FS: writeFile(.ade/attachments/uuid.png)
        Main->>Main: image.resize(96x96) toDataURL()
        Main-->>Renderer: path + previewDataUrl
    else File-backed path
        Renderer->>Renderer: createObjectURL(file)
        Renderer->>Main: IPC: agentChatSaveTempAttachment
        Main->>FS: writeFile(.ade/attachments/uuid.ext)
        Main-->>Renderer: path
    end

    Renderer->>Renderer: onAddAttachment + dropPendingImageAttachment
    Renderer->>Tray: render ImageAttachmentPreview with initialPreviewUrl
Loading

Comments Outside Diff (1)

  1. apps/desktop/src/main/services/ipc/registerIpc.ts, line 3605-3617 (link)

    P2 Legacy fallback embeds full-resolution base64 as preview data URL

    In the legacy path (when saveClipboardImageAttachment is absent), previewDataUrl is constructed as `data:${legacyPayload.mimeType};base64,${legacyPayload.data}` where legacyPayload.data is the raw full-size PNG base64 (up to ~13 MB for a 10 MB image). This string is stored in imagePreviewUrls React state for the lifetime of the attachment. The new IPC path returns a 96×96 thumbnail instead; the fallback silently gives up that optimization and keeps a large string in the renderer heap until the attachment is removed.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/desktop/src/main/services/ipc/registerIpc.ts
    Line: 3605-3617
    
    Comment:
    **Legacy fallback embeds full-resolution base64 as preview data URL**
    
    In the legacy path (when `saveClipboardImageAttachment` is absent), `previewDataUrl` is constructed as `` `data:${legacyPayload.mimeType};base64,${legacyPayload.data}` `` where `legacyPayload.data` is the raw full-size PNG base64 (up to ~13 MB for a 10 MB image). This string is stored in `imagePreviewUrls` React state for the lifetime of the attachment. The new IPC path returns a 96×96 thumbnail instead; the fallback silently gives up that optimization and keeps a large string in the renderer heap until the attachment is removed.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Claude Code

Fix All in Claude Code

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx:1103-1111
Adding `pendingImageAttachments` to the dependency array causes this effect to fire on every pending-attachment state change (every add and every drop), even though the `!composerInputLocked` guard makes each of those extra runs a no-op. Moving the ref-population into the functional state-setter eliminates the closure dependency entirely, so the dep array can stay as `[composerInputLocked]` and the effect only fires on lock transitions.

```suggestion
    clipboardImagePasteFallbackAttachedRef.current = false;
    setPendingImageAttachments((current) => {
      for (const attachment of current) {
        cancelledPendingImageAttachmentsRef.current.add(attachment.id);
      }
      return current.length ? [] : current;
    });
  }, [composerInputLocked]);
```

Reviews (2): Last reviewed commit: "ship: iteration 1 - address pasted image..." | Re-trigger Greptile

@vercel

vercel Bot commented May 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview May 14, 2026 7:51pm

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@arul28 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 39 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 05d5c2b0-6f12-409b-9b71-ba498db3e625

📥 Commits

Reviewing files that changed from the base of the PR and between 72ebd3c and c78517c.

⛔ Files ignored due to path filters (3)
  • docs/ARCHITECTURE.md is excluded by !docs/**
  • docs/features/chat/README.md is excluded by !docs/**
  • docs/features/chat/composer-and-ui.md is excluded by !docs/**
📒 Files selected for processing (10)
  • apps/desktop/src/main/services/adeActions/registry.ts
  • apps/desktop/src/main/services/ipc/registerIpc.ts
  • apps/desktop/src/preload/global.d.ts
  • apps/desktop/src/preload/preload.ts
  • apps/desktop/src/renderer/browserMock.ts
  • apps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx
  • apps/desktop/src/renderer/components/chat/ChatAttachmentTray.test.tsx
  • apps/desktop/src/renderer/components/chat/ChatAttachmentTray.tsx
  • apps/desktop/src/shared/ipc.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/chat-20260514-130223-bc1f1152

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@arul28

arul28 commented May 14, 2026

Copy link
Copy Markdown
Owner Author

@copilot review but do not make fixes

@capy-ai

capy-ai Bot commented May 14, 2026

Copy link
Copy Markdown

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

Comment thread apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx
Comment thread apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx Outdated

Copilot AI commented May 14, 2026

Copy link
Copy Markdown
Contributor

@copilot review but do not make fixes

Reviewed commit b7f3786 without making fixes. I don’t see any blocking issues in the pasted-image preview flow changes, and focused validation passed: AgentChatComposer.test.tsx + ChatAttachmentTray.test.tsx (56 tests).

Copilot finished work on behalf of arul28 May 14, 2026 19:38
@arul28

arul28 commented May 14, 2026

Copy link
Copy Markdown
Owner Author

@copilot review but do not make fixes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants